fix: fix finfo guesser in coroutines#376
Conversation
|
@copilot resolve the merge conflicts in this pull request |
…er-coroutine # Conflicts: # src/foundation/src/Bus/PendingChain.php # src/foundation/src/Support/Providers/RouteServiceProvider.php # src/http/src/Client/PendingRequest.php # src/http/src/Client/Request.php # src/sentry/config/sentry.php # src/telescope/config/telescope.php Co-authored-by: albertcht <9117929+albertcht@users.noreply.github.com>
Co-authored-by: albertcht <9117929+albertcht@users.noreply.github.com>
Resolved in |
The previous fix imported `Hypervel\Context\Context`, which does not exist in this codebase — `hypervel/context` only exposes `CoroutineContext`, `RequestContext`, `ResponseContext`, `ParentCoroutineContext`, `ReplicableContext`. Calling `guessMimeType()` threw a class-not-found error. Switch to `CoroutineContext::getOrSet()` to match how every other ported package uses per-coroutine caching (JwtGuard, Sentry\Hub, UrlGenerator, Log\Context\Repository). Rename the constant to `FINFO_CONTEXT_KEY_PREFIX` per CLAUDE.md naming convention (existing examples: `CACHED_SCHEME_CONTEXT_KEY`, `CACHED_ROOT_CONTEXT_KEY`) and promote it to `public` so the regression test can read the cached `finfo` back out of context.
…tions - Extend `Hypervel\Tests\TestCase` instead of raw `PHPUnit\Framework\TestCase` - Drop the `RunTestsInCoroutine` trait (already on the base class) - Drop `@internal` / `@coversNothing` docblock and `: void` return types - Update fixture path to `Fixtures/` (capital F) Replace the bare `Coroutine::create()` regression test with `parallel()`, which awaits the children and propagates assertion failures back to PHPUnit. The previous form spawned 5 coroutines without a `WaitGroup`, so failures inside them never failed the test. Strengthen the test to actually exercise the per-coroutine isolation the fix provides: each coroutine pulls its cached `finfo` back out of `CoroutineContext` via `FINFO_CONTEXT_KEY_PREFIX` and the test asserts all 5 instances are distinct objects. Reverting to the shared static cache would now fail this test.
|
@albertcht LGTM. I updated the namespaces from 0.3 -> 0.4 (CoroutineContext). I also moved
|
Problem
FileinfoMimeTypeGuessercachedfinfoinstances in a static class property($finfoCache). In a coroutine environment this is unsafe: a singlefinforesource held in a static array is shared across all coroutines, which can lead to race conditions and undefined behavior when multiple coroutines concurrently callguessMimeType().Solution
Replace the static array cache with
Context::getOrSet(), which stores thefinfoinstance in Hypervel's coroutine context. Each coroutine gets its own isolatedfinfoinstance, eliminating the shared-state problem while preserving the caching benefit within a single coroutine's lifetime.The cache key is built from a fixed prefix plus the optional magic file path, matching the previous per-magic-file granularity: